Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Connect to monolith and sort out user auth spagetti #485

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

oscgonfer
Copy link
Collaborator

This PR integrates with the monolith by @timcowlishaw.

The only view left is the map, together with the landing.

The following operations are now passed on to the monolith:

  • Log in (with goto parameter to return back to map/kit page)
  • Sign up
  • My profile
  • Profile
  • Visit user profile
  • Edit button
  • Download CSV
  • SD Card Upload
  • Delete (removed from buttons)
  • Log out

All the pages that could be stored somewhere (edit, profile, users) have a redirect to the monolith as well.

Solving auth spaghetti

In the current app (master) there is an spaghetti in the auth side. There is a bunch of places that request auth data (via stored token) before we actually check if the user is logged in. I fixed this by completely removing the use of localstorage/cookies for auth purposes, and I rely on the request to '/me' resolving. This presents an issue with CORS, that @timcowlishaw has resolved by whitelisting auth requests from our domain. In principle, it's fine, simplifies everything quite nicely (I think) but it'll require quite some testing before deploying.

Pending

Beyond the list above:

  • Cleanup code
  • Solve unhandled error when '/me' returns 401
  • Fix urls to the final /ui path
  • Tweaking on styling to make it match with monolith
  • Testing

Discussion

Now we should discuss whether or not this makes sense or not. Besides the minor issue with CORS, which seems fine, there is a potential issue in that we are touching integral parts of the web, and that it probably makes more sense to transition the map over to the monolith. I think this PR could be there, but I believe that with a bit more of work, we could potentially deprecate this app, and move it all to a more stable stack.

Please, take a look and test, and provide feedback on the different points from above.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant